fix(Router): invalidate compile cache when types change#2
Conversation
…tching Agent-Logs-Url: https://github.com/floatphp/Classes/sessions/336970d1-aa37-441d-b225-35c4429de0c3 Co-authored-by: Jakiboy <7604550+Jakiboy@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a router correctness bug where compile() memoizes compiled route regex by route string only, even though compilation depends on the router’s type map ($this->types). After addTypes() updates types, previously compiled regex patterns could remain stale and cause incorrect matching.
Changes:
- Invalidate the compiled-regex cache when
addTypes()mutates the route type map. - Minor formatting alignment in
addTypes()assignments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function addTypes(array $types) : void | ||
| { | ||
| $this->types = Arrayify::merge($this->types, $types); | ||
| $this->types = Arrayify::merge($this->types, $types); | ||
| $this->cache = []; | ||
| } |
There was a problem hiding this comment.
This change fixes a cache invalidation bug, but there’s no regression test ensuring that calling addTypes() after an initial match() (which populates the compile() cache) causes subsequent match() calls to use the updated type regex. Please add a test that primes the cache with one type definition, then overrides that type via addTypes(), and asserts that matching behavior changes accordingly.
compile()memoizes compiled regex by route string alone, but the output also depends on$this->types. CallingaddTypes()after routes have already been compiled left stale cache entries, causing incorrect pattern matching for any type modified after first use.Change
addTypes()now resets$this->cache = []after merging, ensuring subsequentmatch()calls recompile routes against the updated type map.